feat: add set_provider_and_wait() for blocking initialization#567
feat: add set_provider_and_wait() for blocking initialization#567leoromanovsky wants to merge 1 commit intoopen-feature:mainfrom
Conversation
Summary of ChangesHello @leoromanovsky, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the OpenFeature Python SDK by implementing both blocking and non-blocking provider initialization mechanisms. The primary goal is to meet a key OpenFeature specification requirement and address issues where providers requiring asynchronous setup could not properly signal their readiness. By introducing Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the set_provider_and_wait() function, a valuable addition that aligns the Python SDK with other OpenFeature SDKs and the specification. The implementation is solid, and the new tests provide good coverage. My feedback focuses on a couple of areas for improvement: refactoring some duplicated code in the provider registry for better maintainability, and making the new asynchronous tests more robust by removing time.sleep() in favor of event-based synchronization.
leoromanovsky
force-pushed
the
feat/set-provider-and-wait
branch
from
6ddc22b to
8421e98
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #567 +/- ##
==========================================
- Coverage 98.18% 97.89% -0.30%
==========================================
Files 41 41
Lines 1982 2040 +58
==========================================
+ Hits 1946 1997 +51
- Misses 36 43 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
leoromanovsky
force-pushed
the
feat/set-provider-and-wait
branch
from
8421e98 to
65a0c88
Compare
Adds set_provider_and_wait() to the public API, implementing spec requirement 1.1.2.4: "The API SHOULD provide functions to set a provider and wait for the initialize function to return or abnormally terminate." This is a purely additive change -- set_provider() behavior is unchanged. The only difference with set_provider_and_wait() is that initialization errors are re-raised to the caller. In a future major version, set_provider() should be changed to run initialize() in a background thread to match the non-blocking semantics of Java, Go, and Node.js SDKs. Signed-off-by: Leo Romanovsky <leo.romanovsky@datadoghq.com>
leoromanovsky
force-pushed
the
feat/set-provider-and-wait
branch
from
65a0c88 to
8ddfef0
Compare
|
thanks for the PR, but I don't understand the actual need of this implementation. Currently the provider is synchronically initialized that means when you set a provider it is initialized and you wait till it is done. So, the requirement 1.1.2.4 is currently fulfilled. what doesn't exist is an asynchronous way to do so. we also don't raise an error on purpose and emit a provider error event. |
Understood; it's not ready for review yet I'm working on some ideas for the interface. |
2 participants
Motivation
The Python SDK is the only major OpenFeature SDK missing
set_provider_and_wait(). Java, Go, and Node.js all offer both blocking and non-blocking provider registration. The spec says it SHOULD exist (Requirement 1.1.2.4).Without this, providers that block in
initialize()(waiting for remote config, connecting to a service, etc.) have no way to signal that they're ready before the caller continues. The caller has no SDK-level mechanism to distinguish "provider is initialized and ready" from "set_provider returned but init might still be running."This was surfaced by Datadog's Python OpenFeature provider (FFL-1843), where
initialize()needs to block until Remote Config delivers flag configuration.Changes
Purely additive -- no existing behavior is changed.
set_provider_and_wait(provider, domain?): Same asset_provider()but re-raises initialization exceptions to the caller. Ifinitialize()succeeds,PROVIDER_READYis dispatched and the function returns. Ifinitialize()raises,PROVIDER_ERRORis dispatched AND the exception propagates.set_provider(): Unchanged. Continues to callinitialize()synchronously and swallow exceptions (dispatchingPROVIDER_ERRORon failure).set_provider_and_wait.Decisions
set_provider(). Changing it to run async would be a breaking change for anyone doingset_provider(p); client.evaluate(...). A note in the docstring suggests making it non-blocking in a future major version to match Java/Go/Node.js SDKs._and_waitre-raises exceptions -- this is the only behavioral difference fromset_provider(). It lets callers use try/except to handle initialization failures, matchingsetProviderAndWait()in Java (which throwsOpenFeatureError).Spec Reference